Skip to content

DRAFT: GRPCRoute support #1820

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

DRAFT: GRPCRoute support #1820

wants to merge 4 commits into from

Conversation

ciarams87
Copy link
Contributor

@ciarams87 ciarams87 commented Apr 11, 2024 •
edited
Loading

Proposed changes

This is the draft implementation to get feedback. Unit testing still needs to be completed and some TODOs are still outstanding.

The conformance tests for Method, Hostname, and Header matching are passing (locally only, not running in pipeline yet as the GRPCRoute conformance tests are in the main gateway API branch. They will be released with version 1.1)

    --- PASS: TestConformance/GRPCExactMethodMatching (1.03s)
        --- PASS: TestConformance/GRPCExactMethodMatching/2_request_to_'/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/EchoThree'_should_receive_a_Unimplemented_(12) (0.02s)
        --- PASS: TestConformance/GRPCExactMethodMatching/1_request_to_'/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/EchoTwo'_should_go_to_grpc-infra-backend-v2 (0.03s)
        --- PASS: TestConformance/GRPCExactMethodMatching/0_request_to_'/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/Echo'_should_go_to_grpc-infra-backend-v1 (0.04s)
    --- PASS: TestConformance/GRPCRouteHeaderMatching (0.02s)
        --- PASS: TestConformance/GRPCRouteHeaderMatching/2_request_to_'/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/Echo'_with_headers_'{Color:orange,Version:two}'_should_go_to_grpc-infra-backend-v1 (0.09s)
        --- PASS: TestConformance/GRPCRouteHeaderMatching/6_request_to_'/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/Echo'_with_headers_'{Color:blue}'_should_go_to_grpc-infra-backend-v1 (0.09s)
        --- PASS: TestConformance/GRPCRouteHeaderMatching/7_request_to_'/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/Echo'_with_headers_'{Color:green}'_should_go_to_grpc-infra-backend-v1 (0.09s)
        --- PASS: TestConformance/GRPCRouteHeaderMatching/0_request_to_'/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/Echo'_with_headers_'{Version:one}'_should_go_to_grpc-infra-backend-v1 (0.09s)
        --- PASS: TestConformance/GRPCRouteHeaderMatching/10_request_to_'/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/Echo'_with_headers_'{Color:purple}'_should_receive_a_Unimplemented_(12) (1.04s)
        --- PASS: TestConformance/GRPCRouteHeaderMatching/5_request_to_'/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/Echo'_with_headers_'{Some-Other-Header:one}'_should_receive_a_Unimplemented_(12) (1.04s)
        --- PASS: TestConformance/GRPCRouteHeaderMatching/9_request_to_'/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/Echo'_with_headers_'{Color:yellow}'_should_go_to_grpc-infra-backend-v2 (1.05s)
        --- PASS: TestConformance/GRPCRouteHeaderMatching/8_request_to_'/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/Echo'_with_headers_'{Color:red}'_should_go_to_grpc-infra-backend-v2 (1.07s)
        --- PASS: TestConformance/GRPCRouteHeaderMatching/3_request_to_'/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/Echo'_with_headers_'{Color:blue,Version:two}'_should_go_to_grpc-infra-backend-v2 (1.08s)
        --- PASS: TestConformance/GRPCRouteHeaderMatching/4_request_to_'/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/Echo'_with_headers_'{Color:orange}'_should_receive_a_Unimplemented_(12) (1.10s)
        --- PASS: TestConformance/GRPCRouteHeaderMatching/1_request_to_'/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/Echo'_with_headers_'{Version:two}'_should_go_to_grpc-infra-backend-v2 (1.05s)
    --- PASS: TestConformance/GRPCRouteListenerHostnameMatching (2.11s)
        --- PASS: TestConformance/GRPCRouteListenerHostnameMatching/7_request_to_'no.matching.host/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/Echo'_should_receive_a_Unimplemented_(12) (0.01s)
        --- PASS: TestConformance/GRPCRouteListenerHostnameMatching/6_request_to_'foo.com/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/Echo'_should_receive_a_Unimplemented_(12) (0.01s)
        --- PASS: TestConformance/GRPCRouteListenerHostnameMatching/0_request_to_'bar.com/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/Echo'_should_go_to_grpc-infra-backend-v1 (0.01s)
        --- PASS: TestConformance/GRPCRouteListenerHostnameMatching/1_request_to_'foo.bar.com/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/Echo'_should_go_to_grpc-infra-backend-v2 (0.02s)
        --- PASS: TestConformance/GRPCRouteListenerHostnameMatching/4_request_to_'multiple.prefixes.bar.com/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/Echo'_should_go_to_grpc-infra-backend-v3 (0.05s)
        --- PASS: TestConformance/GRPCRouteListenerHostnameMatching/2_request_to_'baz.bar.com/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/Echo'_should_go_to_grpc-infra-backend-v3 (0.05s)
        --- PASS: TestConformance/GRPCRouteListenerHostnameMatching/5_request_to_'multiple.prefixes.foo.com/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/Echo'_should_go_to_grpc-infra-backend-v3 (0.05s)
        --- PASS: TestConformance/GRPCRouteListenerHostnameMatching/3_request_to_'boo.bar.com/gateway_api_conformance.echo_basic.grpcecho.GrpcEcho/Echo'_should_go_to_grpc-infra-backend-v3 (0.05s)

Closes #ISSUE

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Release notes

If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.

@github-actions github-actions bot added documentation Improvements or additions to documentation enhancement New feature or request dependencies Pull requests that update a dependency file helm-chart Relates to helm chart labels Apr 11, 2024
@ciarams87 ciarams87 force-pushed the feat/grpc-route branch 4 times, most recently from 7379383 to b8fd845 Compare April 12, 2024 14:30
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Apr 12, 2024
@codecov Codecov
Copy link

codecov bot commented Apr 12, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 60.48714% with 292 lines in your changes are missing coverage. Please review.

Project coverage is 82.22%. Comparing base (99cff6e) to head (f3f91aa).
Report is 5 commits behind head on main.

❗ Current head f3f91aa differs from pull request most recent head e39d429. Consider uploading reports for the commit e39d429 to get more accurate results

Files Patch % Lines
internal/mode/static/state/graph/grpcroute.go 3.15% 90 Missing and 2 partials ⚠️
internal/mode/static/state/graph/backend_refs.go 28.23% 60 Missing and 1 partial ⚠️
...ernal/mode/static/state/dataplane/configuration.go 58.97% 45 Missing and 3 partials ⚠️
internal/mode/static/state/graph/route_common.go 86.93% 26 Missing and 6 partials ⚠️
internal/framework/status/setters.go 35.00% 13 Missing ⚠️
internal/framework/status/updater.go 0.00% 10 Missing and 1 partial ⚠️
internal/mode/static/state/dataplane/convert.go 0.00% 10 Missing ⚠️
internal/mode/static/manager.go 46.15% 7 Missing ⚠️
internal/mode/static/nginx/config/servers.go 79.31% 4 Missing and 2 partials ⚠️
internal/framework/status/route.go 61.53% 5 Missing ⚠️
... and 2 more
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1820      +/-   ##
==========================================
- Coverage   86.18%   82.22%   -3.96%     
==========================================
  Files          87       89       +2     
  Lines        5522     5903     +381     
  Branches       52       52              
==========================================
+ Hits         4759     4854      +95     
- Misses        717      991     +274     
- Partials       46       58      +12

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sjberman
Copy link
Collaborator

Can you also update the top level README and our compatibility doc to include GRPCRoute? It's missing from there.

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Apr 15, 2024
@ciarams87 ciarams87 closed this Apr 17, 2024
@ciarams87 ciarams87 deleted the feat/grpc-route branch April 12, 2025 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation enhancement New feature or request helm-chart Relates to helm chart
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ciarams87 @sjberman